-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added pull_request_template.md #299
Added pull_request_template.md #299
Conversation
.github/pull_request_template.md
Outdated
- [ ] I have made corresponding changes to the documentation | ||
- [ ] I have performed a self-review of my own code | ||
- [ ] I have commented my code, particularly in hard-to-understand areas | ||
- [ ] My changes generate no new warnings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, but do you expect people to follow it :) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure whether people will follow it, but its good to have as a reminder to those putting in PRs.
c336a0a
to
56094f8
Compare
@vgvassilev Is this PR ok to merge? |
.github/pull_request_template.md
Outdated
- [ ] I have run `git-clang-format HEAD~` on my changes | ||
- [ ] I have made corresponding changes to the documentation | ||
- [ ] I have performed a self-review of my own code | ||
- [ ] I have commented my code, particularly in hard-to-understand areas |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we collapse these bullets into on that says something similar to “I have read the contribution guide recently” and move these there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and maybe provide a guide to open the PR using labels in the name like:
[ci/Interpreter/docs/tests...] title
Fixes # (issue)
- Tested locally
Test configuration:
should be enough in the PR to make it easy to review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we collapse these bullets into on that says something similar to “I have read the contribution guide recently” and move these there?
@vgvassilev @maximusron Do we have a contributors guide somewhere already, and I just can't find it, or do I need to make one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to make one. Look at the one in Clad.
79bb66b
to
f3d0f21
Compare
3e3207e
to
b00115f
Compare
@vgvassilev I finally got around to finishing this PR added the PR template. I've tried to address the comments you raised, and it's ready for another review. The contributing.md file is basically the same one as in Clad, but with the links changed to the appropriate CppInterOp ones. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This PR adds a basic pull request template to the repo. @vgvassilev @maximusron This PR is ready for review.